Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React 19 #2783

Closed
wants to merge 18 commits into from
Closed

React 19 #2783

wants to merge 18 commits into from

Conversation

alexandernanberg
Copy link
Contributor

@alexandernanberg alexandernanberg commented Jun 12, 2024

Fixes #2756

Copy link

changeset-bot bot commented Jun 12, 2024

🦋 Changeset detected

Latest commit: b190eb3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@react-pdf/renderer Major
@react-pdf/examples Patch
@react-pdf/e2e-node-cjs Patch
@react-pdf/e2e-node-esm Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

package.json Outdated
@@ -46,7 +46,6 @@
"@typescript-eslint/parser": "^6.21.0",
"@vitejs/plugin-react": "^4.2.1",
"babel-plugin-add-module-exports": "^1.0.0",
"canvas": "^2.11.2",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely unable to install this dep, we'll revisit this later

@alexandernanberg alexandernanberg marked this pull request as ready for review June 13, 2024 08:24
@alexandernanberg
Copy link
Contributor Author

alexandernanberg commented Jun 13, 2024

Currently both react-reconciler.development.js and react-reconciler.production.js are included in the bundle, which isn't optimal. I don't see any good alternative though, since jsx-runtime in dev requires dev specific APIs of the reconciler

We could move react-reconciler to a dependency and stop inlining it, but that could cause other issues (duplicate deps etc). Another option is to create separate prod and dev bundles, just like React does. Would be annoying to manage with both esm and cjs tho

@joacub
Copy link

joacub commented Sep 22, 2024

no solutions ?

@diegomura
Copy link
Owner

This is a solution for react19 at the expense of dropping support for all other versions for what I understand. Is that correct?

@alexandernanberg
Copy link
Contributor Author

Yes, that is my understanding. That said, I've not tested this branch with React 18

@Ortieez
Copy link

Ortieez commented Nov 8, 2024

Hi, is there an estimate of when this PR will be merged into the main branch? Our project kind of depends on this. Thanks in advance.

@diegomura
Copy link
Owner

@alexandernanberg is this updated to what's published in npm? I validated this code works for React 19 when installing from npm, but I'm working on a solution to support both the previous versions and 19 at the same time. I tried this code in my solution and there it fails and I'm not seeing the difference

@diegomura
Copy link
Owner

@alexandernanberg here's the branch where I'm pushing that. My idea is to ship both reconcilers, and dynamically use one or the other based on React version. Code there is the one included in this PR, but different structured because of that reason. However, when trying it out I get the following errors.

Did you get those on your testing by any chance? Would you have the time to take a look at my branch? I'm planning to add you as co-author of it anyways :) Thanks in advance

Screenshot 2024-11-15 at 02 45 19

@alexandernanberg
Copy link
Contributor Author

is this updated to what's published in npm?

Yes, but cut a new 4.0.0-canary-5 just to be sure. Still works as expected in my app.

Did you get those on your testing by any chance?

I don't remember exactly, but I did have some issues when trying to use the prod reconciler with dev react or vice versa, the prod bundles are slimmed down and don't include everything. The React packages use dynamic import/requires https://github.com/facebook/react/blob/main/packages/react-reconciler/npm/index.js

I'll take a look at your branch when I have some time to spare!

@diegomura
Copy link
Owner

Will close this in favor of #2941 where I kept support for all the previous React versions too. @alexandernanberg thanks so much for posting this. It was very helpful. Added you as a co-author of the PR although I don't know your email to correctly add the commit message

@diegomura diegomura closed this Nov 16, 2024
@alexandernanberg
Copy link
Contributor Author

Cool, look forward to trying it out! Added my email to my GitHub profile but probably annoying to change it afterwards

@diegomura
Copy link
Owner

Not at all! Just did

Screenshot 2024-11-17 at 10 35 48

Thanks for the help man!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

compat with react 19
4 participants